Skip to content

Conversation

@gfloros
Copy link

@gfloros gfloros commented Jan 15, 2026

No description provided.

Copilot AI review requested due to automatic review settings January 15, 2026 11:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adapts the bop_toolkit to work with industrial datasets by simplifying the codebase and removing support for multiple sensors and modalities per dataset. The changes primarily focus on simplifying dataset parameter handling and removing HOT3D-specific multi-sensor code.

Changes:

  • Simplified dataset parameter handling by removing multi-sensor/multi-modality support
  • Added support for "industrial" dataset with its specific configuration
  • Removed dataset-specific configurations for "xyzibd" and "ipd" datasets
  • Cleaned up logging methods, JSON compression support, and test code

Reviewed changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
setup.py Removed ROS2/Ament package data configuration
scripts/vis_object_symmetries.py Changed default dataset and improved camera parameter retrieval
scripts/vis_gt_poses.py Simplified to remove multi-sensor support, standardized to single annotation format
scripts/vis_est_poses.py Removed HOT3D-specific checks and multi-sensor logic
scripts/eval_calc_scores.py Removed xyzibd-specific configuration and simplified scene parameter calls
scripts/eval_calc_errors_gpu.py Removed dataset-specific configurations and simplified API calls
scripts/eval_calc_errors.py Added industrial dataset config and removed xyzibd-specific settings
scripts/eval_bop24_pose.py Removed custom threshold units and dataset-specific configurations
scripts/eval_bop22_coco.py Simplified scene_tpaths_keys API call
scripts/eval_bop19_pose.py Added industrial dataset configuration
scripts/enumerate_test_targets.py Simplified to remove multi-sensor logic, added commented code
scripts/create_pose_results_file_from_gt.py Fixed logic bug in result generation
scripts/create_coco_results_file_from_gt.py Simplified scene_tpaths_keys call and changed print statement
scripts/calc_model_info.py Fixed 3D bounding box calculation bug
scripts/calc_gt_masks.py Simplified by removing multi-sensor parameter handling
scripts/calc_gt_info.py Removed multi-sensor parameters
scripts/calc_gt_distribution.py Simplified by removing visualization parameters and multi-sensor support
scripts/calc_gt_coco.py Changed datetime call and simplified parameter handling
requirements.txt Fixed Cython version constraint and removed Pillow upper bound
docs/bop_datasets_format.md Removed scene_gt_coco.json from format documentation
bop_toolkit_lib/visualization.py Removed depth difference statistics
bop_toolkit_lib/tests/test_misc.py Removed Precomputer test
bop_toolkit_lib/tests/eval_bop22_coco_test.py Simplified test structure and removed compression support
bop_toolkit_lib/renderer_batch.py Changed worker script path
bop_toolkit_lib/misc.py Fixed Precomputer cache update bug
bop_toolkit_lib/inout.py Removed gzip compression support and simplified JSON I/O
bop_toolkit_lib/dataset_params.py Major refactor removing multi-sensor support and adding industrial dataset

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Refer to the README.md for installation instructions.
""")

logger.warn("""Missing hand_tracking_toolkit dependency,
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logger.warn() method is deprecated in Python. Use logger.warning() instead.

Suggested change
logger.warn("""Missing hand_tracking_toolkit dependency,
logger.warning("""Missing hand_tracking_toolkit dependency,

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +37
ref_pt = map(float, model["pts"].min(axis=0).flatten())
size = map(float, (model["pts"].max(axis=0) - ref_pt).flatten())
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Python 3, map() returns an iterator, not a list. After using ref_pt in line 37 for the calculation, it's consumed. When trying to access ref_pt[0], ref_pt[1], ref_pt[2] in lines 43-45, this will fail with a TypeError. Convert the map objects to lists: ref_pt = list(map(float, model['pts'].min(axis=0).flatten())) and size = list(map(float, (model['pts'].max(axis=0) - ref_pt).flatten())).

Suggested change
ref_pt = map(float, model["pts"].min(axis=0).flatten())
size = map(float, (model["pts"].max(axis=0) - ref_pt).flatten())
ref_pt = list(map(float, model["pts"].min(axis=0).flatten()))
size = list(map(float, (model["pts"].max(axis=0) - ref_pt).flatten()))

Copilot uses AI. Check for mistakes.
Comment on lines +96 to +106
# test_targets_lines = []
# for test_target in test_targets:
# test_targets_lines.append(
# '- {{scene_id: {}, im_id: {}, obj_id: {}, inst_count: {}}}'.format(
# test_target['scene_id'], test_target['im_id'], test_target['obj_id'],
# test_target['inst_count']))

# Save the test targets,
test_targets_path = os.path.join(dp_split["base_path"], p["test_targets_filename"])
# with open(test_targets_path, 'w') as f:
# f.write('\n'.join(test_targets_lines))
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large blocks of commented-out code should be removed. If this code might be needed in the future, it can be retrieved from version control. Dead code reduces maintainability and creates confusion about what is actually being executed.

Suggested change
# test_targets_lines = []
# for test_target in test_targets:
# test_targets_lines.append(
# '- {{scene_id: {}, im_id: {}, obj_id: {}, inst_count: {}}}'.format(
# test_target['scene_id'], test_target['im_id'], test_target['obj_id'],
# test_target['inst_count']))
# Save the test targets,
test_targets_path = os.path.join(dp_split["base_path"], p["test_targets_filename"])
# with open(test_targets_path, 'w') as f:
# f.write('\n'.join(test_targets_lines))
# Save the test targets,
test_targets_path = os.path.join(dp_split["base_path"], p["test_targets_filename"])

Copilot uses AI. Check for mistakes.
"year": datetime.date.today().year,
"contributor": "",
"date_created": datetime.datetime.now(datetime.timezone.utc).isoformat(" "),
"date_created": datetime.datetime.utcnow().isoformat(" "),
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datetime.datetime.utcnow() is deprecated as of Python 3.12. Use datetime.datetime.now(datetime.timezone.utc) instead for timezone-aware UTC timestamps.

Suggested change
"date_created": datetime.datetime.utcnow().isoformat(" "),
"date_created": datetime.datetime.now(datetime.timezone.utc).isoformat(" "),

Copilot uses AI. Check for mistakes.
cmd = [
"python",
"bop_toolkit_lib/call_vsd_worker.py",
"external/bop_toolkit/bop_toolkit_lib/call_vsd_worker.py",
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path to the worker script has been changed from bop_toolkit_lib/call_vsd_worker.py to external/bop_toolkit/bop_toolkit_lib/call_vsd_worker.py. This appears to be an environment-specific path that may not work for all users. The path should be relative to the repository root or use __file__ to construct a path relative to the current module.

Copilot uses AI. Check for mistakes.
"handal": [26, 35, 36, 37, 38, 39, 40],
"ipd": [8, 14, 18, 19, 20],
"xyzibd": [1, 2, 5, 8, 9, 11, 12, 16, 17]
"industrial": [1,3,5]
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent spacing in list formatting. Use consistent spacing after commas: [1, 3, 5].

Suggested change
"industrial": [1,3,5]
"industrial": [1, 3, 5]

Copilot uses AI. Check for mistakes.
"quest3": {"gray1": ".jpg", "gray2": ".jpg"}
"rgb": ".jpg",
"gray1": ".jpg",
"gray2": "jpg",
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing dot prefix in file extension. Should be '.jpg' to match the pattern used for 'rgb' and 'gray1' extensions in lines 442-443.

Suggested change
"gray2": "jpg",
"gray2": ".jpg",

Copilot uses AI. Check for mistakes.
p["elev_range"] = (-0.5 * math.pi, 0.5 * math.pi)

elif dataset_name == "industrial":
p["scene_ids"] = [1,2,3,4,5,6,7,8]
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent spacing in list formatting. Use consistent spacing after commas: [1, 2, 3, 4, 5, 6, 7, 8].

Suggested change
p["scene_ids"] = [1,2,3,4,5,6,7,8]
p["scene_ids"] = [1, 2, 3, 4, 5, 6, 7, 8]

Copilot uses AI. Check for mistakes.
sensor_modalities_have_separate_annotations = {"aria": True, "quest3": True}
p["im_modalities"] = {"aria": ["rgb", "gray1", "gray2"], "quest3": ["gray1", "gray2"]}
modalities_have_separate_annotations = True
p["im_modalities"] = ["rgb","gray1","gray2"]
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent spacing in list formatting. Add spaces after commas: ['rgb', 'gray1', 'gray2'].

Suggested change
p["im_modalities"] = ["rgb","gray1","gray2"]
p["im_modalities"] = ["rgb", "gray1", "gray2"]

Copilot uses AI. Check for mistakes.
from tqdm import tqdm
from bop_toolkit_lib import inout

#
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty comment line should be removed.

Suggested change
#

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants